Skip to content

Conversation

@mu001999
Copy link
Contributor

@mu001999 mu001999 commented Jul 18, 2025

https://github.com/rust-lang/rust/blob/master/compiler/rustc_passes/src/dead.rs#L360-L361 won't insert assoc items into the live set, so that impl items cannot be marked live.

This PR lets impls and impl items can inherit lint levels of the corresponding traits and trait items.

Fixes #144060

r? @petrochenkov

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 18, 2025
Copy link
Contributor

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This behavior seems really non-local and pretty broad. #[allow(dead_code)] on a trait definition suppresses all dead code warnings in all impls with this change?

@mu001999 mu001999 force-pushed the dead-code/allow-trait branch from 7eba1f1 to 7a9e2e2 Compare July 19, 2025 01:34
@mu001999
Copy link
Contributor Author

#[allow(dead_code)] on a trait definition suppresses all dead code warnings in all impls with this change?

@compiler-errors yes, and I think this makes sense. We would have only one trait definition but many impls, letting #[allow(dead_code)] on traits propagate to impls is convenient. Otherwise we need to add #[allow(dead_code)] also on each impls which use unused items.

@petrochenkov
Copy link
Contributor

r? @compiler-errors

@petrochenkov
Copy link
Contributor

Hmm, why did rustbot assign it to me again?
I'll just block this on #144863 now.
@rustbot blocked

@rustbot rustbot added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 4, 2025
@mu001999
Copy link
Contributor Author

mu001999 commented Aug 4, 2025

Hmm, why did rustbot assign it to me again?

I tried reassigning, but rustbot didn’t respond after waiting a while, so I deleted the comment. 😂

@cjgillot cjgillot self-assigned this Aug 4, 2025
@petrochenkov petrochenkov removed their assignment Aug 5, 2025
@bors
Copy link
Collaborator

bors commented Aug 5, 2025

☔ The latest upstream changes (presumably #144863) made this pull request unmergeable. Please resolve the merge conflicts.

@mu001999 mu001999 force-pushed the dead-code/allow-trait branch from 7a9e2e2 to c1be949 Compare August 7, 2025 16:34
@mu001999
Copy link
Contributor Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Aug 10, 2025
@tgross35
Copy link
Contributor

@cjgillot gentle ping for a review here

@wesleywiser wesleywiser added the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Sep 25, 2025
@wesleywiser
Copy link
Member

Nominated the issue for lang team discussion, let's see what they think before merging.

@mu001999
Copy link
Contributor Author

mu001999 commented Sep 25, 2025

oh, I missed the non-local in the note from @compiler-errors

and if the trait is non-local, the dead-code lint level won't spread to the local impls. cc @wesleywiser

we only care about local traits, see https://github.com/rust-lang/rust/pull/144113/files#diff-719b4f36bbbf3b3bf48683429408f5d27e5633f70c1144a93554bd536d5bf655R786

@traviscross traviscross added T-lang Relevant to the language team I-lang-nominated Nominated for discussion during a lang team meeting. I-lang-radar Items that are on lang's radar and will need eventual work or consideration. labels Oct 8, 2025
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 28, 2025
@jdonszelmann
Copy link
Contributor

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Jan 6, 2026

📌 Commit 12474ce has been approved by jdonszelmann

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 6, 2026
@bors bors merged commit 8b1bf8a into rust-lang:main Jan 6, 2026
11 checks passed
@rustbot rustbot added this to the 1.94.0 milestone Jan 6, 2026
@JonathanBrouwer
Copy link
Contributor

Candidate for the perf regression in #150726
@rust-timer build bb5b182

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (bb5b182): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @rustbot label: +perf-regression-triaged. If not, please fix the regressions and do another perf run. If its results are neutral or positive, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)
0.4% [0.2%, 0.8%] 13
Regressions ❌
(secondary)
0.3% [0.3%, 0.3%] 3
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.6% [-0.6%, -0.5%] 3
All ❌✅ (primary) 0.4% [0.2%, 0.8%] 13

Max RSS (memory usage)

Results (primary -0.6%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.6% [-0.6%, -0.6%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -0.6% [-0.6%, -0.6%] 1

Cycles

Results (secondary 4.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
4.1% [3.2%, 5.0%] 2
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 473.921s -> 475.634s (0.36%)
Artifact size: 390.82 MiB -> 390.83 MiB (0.00%)

@rustbot rustbot added the perf-regression Performance regression. label Jan 6, 2026
@JonathanBrouwer
Copy link
Contributor

Yeah seems like this PR is the cause sadly, what action should we take to resolve this?

@Mark-Simulacrum
Copy link
Member

I'm going to mark as triaged. This only significantly affected bitmaps in the non-incremental scenarios, and the change looks fairly minimal to me -- not clear that it could be made any better -- so I don't think there's much to be done here given this is effectively a new feature.

@Mark-Simulacrum Mark-Simulacrum added perf-regression-triaged The performance regression has been triaged. relnotes Marks issues that should be documented in the release notes of the next release. labels Jan 12, 2026
@mu001999 mu001999 deleted the dead-code/allow-trait branch January 12, 2026 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. I-lang-radar Items that are on lang's radar and will need eventual work or consideration. needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. perf-regression Performance regression. perf-regression-triaged The performance regression has been triaged. relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team to-announce Announce this issue on triage meeting

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Lint regression: dead_code ignores #[allow(dead_code)] on traits